Conversation
There was a problem hiding this comment.
Tested on Android emulator--looks good! Admittedly I'm not as familiar with the notification system so would probably be best if someone who has more experience with it could look this over too.
Just one small request--I noticed that when you edit your previous reminder settings, the dialog box shows you which buses you selected previously, but the slider always starts at 5 minutes. Could you make sure the dialog "remembers" the slider position (in addition to the checkboxes)?
Thanks!
| for (final reminder in await _activeReminders(token: token)) { | ||
| for (int i = 0; i < modifications.length; ++i) { | ||
| if (reminder.stpid == modifications[i].encode()["stpid"] && reminder.rtid == modifications[i].encode()["rtid"]) { | ||
| if (added[i] is RemoveReminder) throw Exception("backend not updated"); |
There was a problem hiding this comment.
The logic here seems wrong, added is a list of bools so its elements can't ever be of type RemoveReminder. Just removing that line should fix it.
|
|
||
| for (final reminder in await _activeReminders(token: token)) { | ||
| for (int i = 0; i < modifications.length; ++i) { | ||
| if (reminder.stpid == modifications[i].encode()["stpid"] && reminder.rtid == modifications[i].encode()["rtid"]) { |
There was a problem hiding this comment.
I recommend adding getters to RemindersModification (and its subclasses as necessary) so you don't have use encode to do this.
| Navigator.pop(context); | ||
| } on Exception catch (e) { | ||
| } on Exception { | ||
| showDialog( |
There was a problem hiding this comment.
This should probably be using the maizebus dialog.
jumpy-cat
left a comment
jumpy-cat
left a comment
There was a problem hiding this comment.
I tested it against simulated backend issues and it appears to work great! Did find a few small issues that you should address, check the comments I left.
Out of scope for this pr but now that the active list of reminders is fetched after each modification, the reminder widgets could be directly updated and the backend would be able to send less push messages. I've added a note to the reminders tracking issue.
3 participants
Description
When setting notification requests for a stop, the app checks to make sure the notification list has the reminders to be added and omits the reminders to be deleted. Otherwise, a popup appears telling the user the notification request could not be sent.
Type of Change
feat)fix)Changes Made
stop_sheet.dartnow displays a different error messagemodifyReminders()inincoming_bus_reminder_service.dartnow checks backendTesting Done
Flutter:
flutter analyzeandflutter formatpassedflutter test)Backend:
Integration:
Checklist
[type](scope): short descriptionprint()/console.log()left in production code